feat: support passing AbortSignal instances to the configured blockservice #267
Conversation
|
Refs ipfs/js-ipfs#2884 |
This is so the user can signal that they are no longer interested in the results of the operation and system components can stop trying to fulfil it.
achingbrain
force-pushed
the
feat/support-aborting-requests
branch
from
8ef58c4
to
c9fe11c
Compare
Codecov Report
@@ Coverage Diff @@
## master #267 +/- ##
==========================================
+ Coverage 86.84% 90.13% +3.28%
==========================================
Files 2 2
Lines 152 152
==========================================
+ Hits 132 137 +5
+ Misses 20 15 -5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I've just one question (inline).
This one seems to need ipfs/js-ipfs-block-service#89 to be released before it should be merged.
| @@ -335,7 +348,7 @@ class IPLDResolver { | |||
| if (treePaths.length === 0 && queue.length > 0) { | |||
| ({ cid, basePath } = queue.shift()) | |||
| const format = await this._getFormat(cid.codec) | |||
| block = await this.bs.get(cid) | |||
| block = await this.bs.get(cid, options) | |||
There was a problem hiding this comment.
Would it make sense to forward only the signal value and not all of the options object? This is really just a question.
There was a problem hiding this comment.
It's a fair point, but the way we wrap modules inside modules, if we let the wrapping module be the gatekeeper of what gets passed on, it becomes tied to the API of that module more strongly.
This may cause problems down the line as we're potentially passing more arguments to the wrapped module than strictly necessary, but we trade off against flexibility in that case.
vmx
force-pushed
the
feat/support-aborting-requests
branch
from
c9fe11c
to
5661753
Compare
|
The test failure seems to be an actual one. I can reproduce it locally even with a 30s timeout. I did run it locally with: It doesn't happen all the time, but almost on every run I do. @achingbrain can you please have a look? |
achingbrain
force-pushed
the
feat/support-aborting-requests
branch
from
5661753
to
6488b8a
Compare
|
Looks better now. Did you force-push just to trigger a build? Cos you can do that from the Travis CI without changing public history.. |
|
I force-pushed because I did a rebase. |
This is so the user can signal that they are no longer interested in the results of the operation and system components can stop trying to fulfil it.